Skip to content

feat: "Ran while you were away" card on /studies (feat_overnight_studies_summary_card)#444

Merged
SoundMindsAI merged 12 commits into
mainfrom
feat/overnight-studies-summary-card
Jun 4, 2026
Merged

feat: "Ran while you were away" card on /studies (feat_overnight_studies_summary_card)#444
SoundMindsAI merged 12 commits into
mainfrom
feat/overnight-studies-summary-card

Conversation

@SoundMindsAI
Copy link
Copy Markdown
Owner

Summary

Ships the "Ran while you were away" card on /studies — the index-page surface for the overnight autopilot (the chain panel + <OvernightResultCard> already cover the detail page).

  • Backend — new read-only discovery endpoint GET /api/v1/studies/chains/recent returning the deduplicated set of recently-completed overnight chains (length ≥ 2, terminal, optional ?since= cutoff). Repo helper list_recent_completed_chains reuses the Phase 1 get_chain_for_study traversal and derive_chain_stop_reason — no chain math is re-derived. Inert pagination per OQ-2 (next_cursor: null, has_more: false, X-Total-Count = len(data)).
  • Frontend — new RecentChainsCard component above the studies table, owning its own data via two new hooks: useRecentChains(since) (TanStack) and useStudiesVisited() (localStorage, +1ms exclusive dismissal nudge per FR-5). Stop-reason phrasing reuses CHAIN_STOP_REASON_PHRASE from ui/src/lib/chain-stop-reason.ts — the same map shipped with feat_overnight_final_solution_phase2, so the card and the chain panel never drift. New recent_chains_card glossary key.
  • Tests — 7 integration (Story 1.1 repo helper) + 4 integration (Story 1.2 endpoint) + 9 contract (Story 1.2 response shape + 422 envelope) + 4 vitest (Story 2.1 hook) + 8 vitest (Story 2.2 card) + 2 Playwright (Story 3.1 E2E, smoke-job opt-in). Full UI suite green (1115 vitest).
  • Docsapi-conventions.md + ui-architecture.md updated per Story 3.2.

No migration. All chain math is derived from existing schema; Alembic head stays at 0022_solr_engine_auth_check.

Verdict tables (cross-model adjudication)

GPT-5.5 Epic 1 review (1 finding)

# Sev Location Verdict Notes
1 Medium backend/app/api/v1/studies.py:631 Rejected Claimed select_best_link returns a link object. Counter-evidence: backend/app/domain/study/chain_summary.py:212 typed `-> str

GPT-5.5 Epic 2 review (2 findings)

# Sev Location Verdict Notes
1 Medium ui/src/components/studies/recent-chains-card.tsx:57 Accepted Fixed in 057c6168 — replaced ?? row.stop_reason raw-enum fallback with ?? 'Chain stopped' so a forward-compat backend drift renders a generic phrase instead of leaking the raw wire value.
2 Medium ui/src/components/studies/recent-chains-card.tsx:58 Rejected Claimed the null-metric gate should include cumulative_lift. Counter-evidence: plan §3 Story 2.2 inventory + §11.4 consistency review gate the null branch on best_metric === null only; the existing chain panel at auto-followup-chain-panel.tsx:259 renders formatSignedLift returning "—" for null lift, which is the project's universal null-cell convention. My code matches the planned gate verbatim.

Test plan

  • make lint && make typecheck (backend, clean)
  • ./.venv/bin/ruff format --check backend/ (573 files formatted)
  • Backend unit tests: 2224 passed
  • Backend integration (make test-worktree): 8 + 4 new tests pass (Story 1.1 repo + Story 1.2 endpoint)
  • Backend contract: 9 new tests pass (Story 1.2)
  • Frontend pnpm test — 1115 vitest passed (151 files)
  • Frontend pnpm typecheck — clean
  • Frontend pnpm lint — 0 errors (176 pre-existing warnings)
  • Frontend pnpm build — clean (/studies still ○ static)
  • Smoke E2E job — SMOKE_TEST is OFF by default per state.md. The new spec at ui/tests/e2e/recent-chains-card.spec.ts typechecks; runtime green is gated on operator opt-in (gh variable set SMOKE_TEST --body true).
  • Manual visual check on /studies after merge — does the card render correctly with seeded chains?

Deferred work captured (per plan §1)

Two 99_backlog/ idea files filed for the spec's open questions (defer-until-incident):

Guide impact

Guide 06 (Create and monitor a study) captures 01-studies-list.png of /studies. The new RecentChainsCard renders above the table when there are recent terminated chains; the Acme seed in guide 06's setup may or may not trigger the card depending on chain tail vs the 7-day default cutoff. Flagged for operator-decided regen — the card is deterministic enough that the operator can choose to capture it (and localStorage.setItem a known cutoff) or hide it (set the cutoff to now).

Notes

Commit 04732efd docs(planned): idea — walkthrough guides on public website (relyloop.com) landed on this branch from a parallel session — it's an unrelated planned-feature idea file. Disclosing rather than reverting (the commit is coherent on its own).

🤖 Generated with Claude Code

SoundMindsAI and others added 10 commits June 4, 2026 05:12
Add the pure-read discovery helper backing the upcoming
GET /api/v1/studies/chains/recent endpoint (FR-1):

- list_recent_completed_chains(db, *, since, limit) returns
  deduplicated completed overnight chains (length >= 2), newest
  tail-completion first, capped at `limit` distinct chains
- Reuses Phase 1's get_chain_for_study traversal and
  derive_chain_stop_reason rather than re-deriving chain math
- Filters: parent_study_id IS NOT NULL (length >= 2 implied),
  completed_at IS NOT NULL, terminal status, optional since-cutoff
- Scan-caps candidates at limit * _CHAIN_MAX_DESCENDANTS so dedup-to-
  anchor can fill `limit` distinct chains in the maxed-out case
- Defensive in-flight skip on resolved traversals (interior link
  still running -> exclude)
- Concurrent-delete safety: skip candidates whose traversal returns
  None, never raise

8 integration tests (AC-1/2/3/4/11/12 + concurrent-delete safety net
via monkeypatching).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
Adds the read-only discovery endpoint backing the upcoming "Ran while
you were away" card (FR-1, AC-1/5/6/11):

- RecentChainSummary + RecentChainsResponse Pydantic schemas (10 + 3
  fields). Reuses ObjectiveDirection + ChainStopReason Literals so the
  source-of-truth comment chain stays intact.
- get_recent_chains handler declared BEFORE /studies/{study_id} so the
  static "chains" path segment is not captured as a dynamic study_id —
  route-order regression asserted by an integration test.
- _recent_chain_row helper mirrors get_study_chain's derivation block
  (select_best_link + compute_cumulative_lift + derive_chain_stop_reason)
  without extracting a shared helper (plan §5 — bounded refactor only).
- Typed query params (since: datetime | None, limit: int ge=1 le=50
  default 20) auto-route malformed input through the global
  validation_exception_handler -> 422 VALIDATION_ERROR envelope.
- Emits X-Total-Count = len(data); inert pagination
  (next_cursor=null, has_more=false) per OQ-2 resolution.

Tests: 4 integration (AC-1, AC-5, AC-11, route-order regression) + 9
contract (response-model shape, enum literals, OpenAPI presence,
X-Total-Count header, 422 envelope for malformed since/limit).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
New glossary entry backing the InfoTooltip on the upcoming "Ran while
you were away" card on /studies (FR-6). short=120 chars (≤140 cap),
long=~720 chars; both pass the existing length-cap test. Lands first
because <InfoTooltip glossaryKey="..."> is type-locked against
ShortGlossaryKey — the card in Story 2.2 won't compile without it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
…com)

Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
Adds the two TanStack/localStorage hooks the upcoming RecentChainsCard
will consume (FR-1, FR-5):

- useStudiesVisited(): returns { since, dismiss }. since is the
  ISO-8601 cutoff read from localStorage or now - 7d on first visit
  (AC-9). dismiss(T) writes T + 1ms so the inclusive since filter
  doesn't re-show the just-dismissed chain (FR-5). Defensive: malformed
  input is ignored without throwing. SSR-safe via typeof window guard.

- useRecentChains(since, opts?): TanStack hook fetching the new
  GET /api/v1/studies/chains/recent endpoint. Refetches on window
  focus + reconnect only — best-effort discoverability, no aggressive
  polling. Mirrors useStudyChain shape.

Tests: 4 vitest cases on useStudiesVisited covering AC-8 (dismiss+1ms),
AC-9 (7d default), persistence across mounts, and defensive parse.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
…2.2)

Adds the "Ran while you were away" card above the studies table on
/studies (FR-1, FR-3, FR-4, FR-6, ACs 7/8/10/11):

- <RecentChainsCard>: self-contained card consuming useStudiesVisited
  + useRecentChains from Story 2.1. Early-returns null on pending /
  error / empty so the studies table beneath always renders predictably
  (best-effort discoverability per spec §10 failure modes).
- Per-row layout: anchor name as a <Link href=/studies/{anchor_study_id}>,
  chain length, "Best <metric>: <value>" line, "Lift: <±value>" via
  the shared formatSignedLift helper, stop-reason phrase via the shared
  CHAIN_STOP_REASON_PHRASE map (both already extracted by
  feat_overnight_final_solution_phase2 Story 1 / FR-8).
- Null-metric branch (AC-11): when best_metric is null the card drops
  the numeric line entirely and leads with the stop-reason phrase
  instead of rendering "Best ndcg: —".
- "Got it" computes max(tail_completed_at) across rows and calls
  dismiss(maxIso); the +1ms exclusive nudge in useStudiesVisited
  prevents the inclusive since filter from re-showing the just-
  dismissed chain (FR-5).
- TWO <InfoTooltip> affordances per FR-6: recent_chains_card on the
  CardTitle + overnight_autopilot reused on the "Overnight" label.

Mount: /studies/page.tsx renders <RecentChainsCard /> immediately
after the page header, before the target-filter chip row. No
shared-state changes — the card owns its own query + visited-state.

Tests: 8 vitest cases covering pending/error/empty -> null; AC-7
full-row render; AC-10 all 6 stop_reason wire values phrased; AC-11
null-metric drops numeric line; AC-8 dismiss with max tail; FR-6 both
InfoTooltip affordances present. Full ui suite 1115 vitest green, tsc
clean, Next.js build clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
GPT-5.5 phase-gate review flagged the `?? row.stop_reason` fallback in
the stop-reason phrasing as a potential raw-enum leak under backend
forward-compat drift. The Record<ChainStopReason, string> typing makes
the lookup exhaustive at compile time, but if the backend ships a new
wire value before the frontend redeploys, the user would see e.g.
`Stopped: no_lift_v2` instead of a phrase. Replace the fallback with
the generic 'Chain stopped' phrase — same UX outcome, no leaked enum.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
Adds the smoke-job E2E that exercises the "Ran while you were away" card
end-to-end against the running stack (FR-1 + AC-7 + AC-8):

- Test 1 ("renders the card with a working Review chain link"):
  seeds a 3-link chain via seedAutoFollowupChain with inFlightLeaf=false
  so the chain is terminal, sets localStorage["relyloop.last_visited_
  studies_at"] to 2000-01-01 via page.addInitScript(), navigates to
  /studies, asserts the card and its anchor link, then clicks through
  and asserts navigation to /studies/{root_id} and the study page summary.
- Test 2 ("'Got it' dismisses ... stays hidden after reload"): same
  setup, clicks the dismiss button, asserts the card unmounts on next
  refetch, reloads, asserts the card stays gone (FR-5 — the +1ms
  exclusive nudge in useStudiesVisited prevents re-show across the
  reload).

Per CLAUDE.md E2E rules: no page.route() mocking of backend; setup via
seed helpers (request only); assertions via the page object. Card
visibility / null-metric / phrase mapping / stop-reason exhaustiveness
are covered in detail by the vitest component suite — this spec proves
the real-stack mount + navigation flow.

Note: the smoke job is OFF by default per state.md (SMOKE_TEST=false);
operators opt in via `gh variable set SMOKE_TEST --body true`. Spec
typechecks via `pnpm typecheck`; runtime green is gated on operator opt-in.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
…(Story 3.2)

- api-conventions.md: new paragraph on read-only discovery endpoints
  with inert pagination, citing GET /api/v1/studies/chains/recent as
  the first entry. Documents X-Total-Count = len(data), the always-
  null next_cursor + always-false has_more wire contract (forward-
  compat with a possible MVP3 keyset story), and the route-order
  ordering requirement (static "chains" declared before the dynamic
  {study_id} route).
- ui-architecture.md: extends the /studies row in the routes table
  with a description of the dismissible RecentChainsCard above the
  table — owns its own data via useRecentChains + useStudiesVisited,
  early-returns null on pending/error/empty (best-effort discoverability),
  reuses CHAIN_STOP_REASON_PHRASE from the shared
  ui/src/lib/chain-stop-reason.ts so the card + the chain panel stay
  aligned on a single source of truth.

state.md / state_history.md updates land at finalization (post-merge,
per Story 3.2 DoD + post-impl Step 2).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
…ains card

Per feat_overnight_studies_summary_card implementation_plan.md §1
"Deferred phases": the spec's open questions OQ-2 (keyset pagination on
GET /api/v1/studies/chains/recent) and OQ-3 (Postgres indexes on
studies.parent_study_id + studies.completed_at) are both defer-until-
incident — not blocking the v1 ship, but worth capturing so future
operators (or the next agent) don't have to re-derive the analysis.

Both land in 99_backlog/ (defer-until-incident classification — not
the active MVP2 release bucket — because:
- the discovery endpoint emits inert pagination by design under the
  hard limit ≤ 50 cap (OQ-2 resolved limit-cap-only for v1);
- the discovery query reads ≤ 100 candidates at default sizing,
  trivial at single-tenant on-laptop scale (OQ-3).

Each idea file cites the spec OQ, names the trigger event that
should pull it forward, scopes the work (capabilities + backend /
frontend / migration / config impact), and cross-links the two
ideas (they pair cleanly — keyset pagination needs the indexes).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements the 'Ran while you were away' feature, introducing a new backend endpoint GET /api/v1/studies/chains/recent and a frontend <RecentChainsCard> component to display recently completed overnight chains. Feedback on the backend highlights an N+1 query pattern in the chain discovery repository helper, suggesting a set-based tracking mechanism to skip redundant traversals. On the frontend, the reviewer identified a potential hydration mismatch in Next.js caused by reading from localStorage during the initial render of the useStudiesVisited hook, recommending a useEffect approach for SSR safety along with defensive validation of the retrieved date string.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +450 to +458
by_anchor: dict[str, ChainTraversalResult] = {}
for candidate_id in candidate_ids:
if len(by_anchor) >= limit:
break
traversal = await get_chain_for_study(db, candidate_id)
if traversal is None:
# Concurrent hard-delete between candidate query and traversal
# (e.g. test teardown). Skip silently per Story 1.1 task 5.
continue
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current implementation of list_recent_completed_chains suffers from an N+1 query pattern. It iterates over all candidate IDs and calls get_chain_for_study for each one. Since multiple candidates can belong to the same chain, this results in redundant database queries and traversal calculations for the same chain. Keeping track of already traversed study IDs in a set allows us to skip redundant calls entirely, significantly improving performance.

    by_anchor: dict[str, ChainTraversalResult] = {}
    seen_study_ids: set[str] = set()
    for candidate_id in candidate_ids:
        if len(by_anchor) >= limit:
            break
        if candidate_id in seen_study_ids:
            continue
        traversal = await get_chain_for_study(db, candidate_id)
        if traversal is None:
            # Concurrent hard-delete between candidate query and traversal
            # (e.g. test teardown). Skip silently per Story 1.1 task 5.
            continue
        seen_study_ids.update(lk.id for lk in traversal.links)

* (`ui/src/hooks/use-local-storage-set.ts`).
*/

import { useCallback, useState } from 'react';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Import useEffect to handle reading from localStorage safely after mount, preventing hydration mismatches.

Suggested change
import { useCallback, useState } from 'react';
import { useCallback, useEffect, useState } from 'react';

Comment on lines +68 to +72
export function useStudiesVisited(): UseStudiesVisitedResult {
// Hydrate synchronously via the useState initializer — matches the
// pattern in useLocalStorageSet (no extra render cycle, safe under
// SSR because readVisitedAt() guards on `typeof window`).
const [since, setSince] = useState<string>(() => readVisitedAt());
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Initializing state directly from localStorage during the initial render causes a hydration mismatch in Next.js because the server renders with defaultSince() while the client hydrates with the value from localStorage. To ensure SSR safety, initialize the state to defaultSince() and update it in a useEffect hook on mount.

Suggested change
export function useStudiesVisited(): UseStudiesVisitedResult {
// Hydrate synchronously via the useState initializer — matches the
// pattern in useLocalStorageSet (no extra render cycle, safe under
// SSR because readVisitedAt() guards on `typeof window`).
const [since, setSince] = useState<string>(() => readVisitedAt());
export function useStudiesVisited(): UseStudiesVisitedResult {
const [since, setSince] = useState<string>(() => defaultSince());
useEffect(() => {
setSince(readVisitedAt());
}, []);

Comment on lines +43 to +52
function readVisitedAt(): string {
if (typeof window === 'undefined') return defaultSince();
try {
const raw = window.localStorage.getItem(STORAGE_KEY);
if (raw) return raw;
} catch {
// Private browsing / quota / corrupt — fall back to default.
}
return defaultSince();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Defensively validate the value retrieved from localStorage to ensure it is a valid date string before returning it. This prevents sending malformed parameters to the backend and triggering 422 errors.

Suggested change
function readVisitedAt(): string {
if (typeof window === 'undefined') return defaultSince();
try {
const raw = window.localStorage.getItem(STORAGE_KEY);
if (raw) return raw;
} catch {
// Private browsing / quota / corrupt — fall back to default.
}
return defaultSince();
}
function readVisitedAt(): string {
if (typeof window === 'undefined') return defaultSince();
try {
const raw = window.localStorage.getItem(STORAGE_KEY);
if (raw && !Number.isNaN(Date.parse(raw))) return raw;
} catch {
// Private browsing / quota / corrupt — fall back to default.
}
return defaultSince();
}

SoundMindsAI and others added 2 commits June 4, 2026 05:55
CI's test_openapi_has_no_orphan_endpoints test (the hand-maintained
EXPECTED_ENDPOINTS allowlist guarding the OpenAPI surface from
drift) tripped on the new GET /api/v1/studies/chains/recent endpoint
shipped by feat_overnight_studies_summary_card Story 1.2.

Plan §3.3 explicitly flagged this: "Coordinate with
bug_contract_allowlists_outdated_after_mvp2_features — if a hand-
maintained endpoint/route allowlist trips on the new route, update
it in the same PR." Adding the entry alongside the existing
/studies/{study_id}/chain row (the Phase 1 chain endpoint).

Local: 131 contract tests pass in the openapi-surface suite.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
- Backend (finding #1, High): eliminate N+1 in list_recent_completed_
  chains. Track seen_study_ids across the candidate loop so candidates
  belonging to a chain already resolved skip the redundant
  get_chain_for_study call. Worst case for a 6-link chain drops from 6
  traversal calls to 1. The dedup outcome is unchanged.
- Frontend (finding #4, Medium): defensively validate the localStorage
  value in useStudiesVisited's readVisitedAt(). A corrupt stored value
  (operator manual edit, older-release shape) would otherwise propagate
  to GET /api/v1/studies/chains/recent?since=<garbage> and 422 every
  request until the operator clears localStorage. Now silently falls
  back to the 7-day default.

Findings #2 + #3 (High, claimed SSR hydration mismatch from useState
localStorage read) rejected — see PR-444 adjudication comment for
counter-evidence: the card returns null while query.data is undefined
so the DOM output is identical on server (no fetch) and client
(initial render), and the useState-initializer + typeof window pattern
is the project's established convention (use-local-storage-set.ts
:46-48 uses the exact same shape).

Tests: 21 backend + 12 vitest pass against the updated code.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
@SoundMindsAI
Copy link
Copy Markdown
Owner Author

Review adjudication (Gemini Code Assist)

Commits landing fixes: fa8a13f5

Gemini Code Assist (4 findings)

# Sev Location Verdict Notes
1 High backend/app/db/repo/study.py:458 Accepted Fixed in fa8a13f5 — added seen_study_ids set so candidates belonging to a chain already resolved skip the redundant get_chain_for_study call. Worst case for a 6-link chain drops from 6 traversal calls to 1. Dedup outcome unchanged.
2 High ui/src/hooks/use-studies-visited.ts:34 Rejected Paired with finding #3 — would add a useEffect import only if #3 was accepted. Rejected as a consequence of #3 being rejected.
3 High ui/src/hooks/use-studies-visited.ts:72 Rejected Claimed SSR hydration mismatch from reading localStorage in the useState initializer. Counter-evidence: (a) the card returns null while query.data is undefined (isPending) — the DOM output is identical on server (no fetch) and on the client's initial render, so no hydration mismatch is possible; (b) the useState-initializer + typeof window === 'undefined' guard pattern is the project's established convention — ui/src/hooks/use-local-storage-set.ts:46-48 uses the same shape with the documented comment "Safe under SSR because the read function checks typeof window." Accepting #3 would diverge from the established codebase pattern.
4 Medium ui/src/hooks/use-studies-visited.ts:52 Accepted Fixed in fa8a13f5 — added !Number.isNaN(Date.parse(raw)) guard so a corrupt localStorage value (operator manual edit, older-release shape) doesn't propagate to GET /api/v1/studies/chains/recent?since=<garbage> and 422 every request until localStorage is cleared.

Outcomes

Ready for human review + merge.

@SoundMindsAI
Copy link
Copy Markdown
Owner Author

Final cross-model review (GPT-5.5)

Convergence note: 0 findings — clean pass.

Reviewed: full PR diff (git diff origin/main..HEAD, 542 lines across backend repo/endpoint/schema, frontend hooks/card/page, glossary, tests, docs, deferred-work idea files).

Rejection log carried forward from prior phases (Epic 1 + Epic 2 GPT-5.5, Gemini #2/#3) was supplied as a no-re-raise hint; final review found no new High/Medium issues to surface and did not re-raise any rejected findings.

CI is green on fa8a13f5 (the head after Gemini fixes). Ready for human review + merge.

@SoundMindsAI SoundMindsAI merged commit ba1e6d6 into main Jun 4, 2026
18 checks passed
@SoundMindsAI SoundMindsAI deleted the feat/overnight-studies-summary-card branch June 4, 2026 10:29
SoundMindsAI added a commit that referenced this pull request Jun 4, 2026
…445)

- Move feature folder 02_mvp2/ -> implemented_features/2026_06_04_*
  (spec + plan + idea + pipeline_status travel together; no phase
  idea files to preserve).
- pipeline_status.md: Implementation -> Complete (PR #444, ba1e6d6,
  7/7 stories, 33 tests, cross-model review summary); add Release: mvp2
  marker so the dashboard classifier keeps it in MVP2 after the
  bucket-less move.
- implementation_plan.md: Status -> Complete (PR #444, merged 2026-06-04).
- state.md: new merge prepended to Last 5; PR #433 dropped to the
  older-entries reference line; branch/active-feature/in-flight/queued
  refreshed. state.md = 26 KB (under 60 KB gate).
- state_history.md: full merge narrative prepended.
- Dashboards (MVP2 + backlog) regenerated by pre-commit hook.

Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SoundMindsAI added a commit that referenced this pull request Jun 4, 2026
…proposal_full_param_space_view) (#446)

* docs(idea): apply preflight-2026-06-04 patches to feat_proposal_full_param_space_view

Lands the audit-and-patch edits from a prior /idea-preflight run that had
been sitting in the working tree, plus the dashboard regen that follows.

- Header: "preflight-refreshed 2026-06-04" + linkified sibling references
  to the now-implemented overnight trio (PR #440 / #442 / #444).
- Cap 1 grounds the panel target on <ConfigDiffPanel> at
  ui/src/components/proposals/config-diff-panel.tsx:63 (replacing the
  "Recommended config" placeholder which doesn't match the live tree).
- declared_params shape correction: flat Record<string, type-tag>, NOT
  per-param bounds/defaults — bounds live on study.search_space.
- Cap 2 coordination note pointing at the existing parent-vs-swap-target
  diff in <SuggestedFollowupsPanel> (suggested-followups-panel.tsx:250-291)
  so the new panel reuses the established visual grouping.
- Scope signals: "backend: none required" — confirmed the proposal page
  already pays for useTemplate(parentStudy.template_id) at
  ui/src/app/proposals/[id]/page.tsx:183; chain-link proposals inherit
  template_id from parent (backend/workers/auto_followup.py), so the
  template's declared_params is on-page for free.
- Q2 + Q3 rewritten against the corrected data shapes.
- MVP2_DASHBOARD.md status cell picks up the linkified
  feat_overnight_final_solution reference.

No new content; this is the preflight audit-and-patch result. Spec
generation runs next.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>

* docs(spec): feat_proposal_full_param_space_view — three-state full-param-space panel on proposals

Generates the feature specification + pipeline_status.md from the
preflight-refreshed idea.md. Converged across 3 GPT-5.5 cycles + 1 Opus
internal verification pass.

The spec adds a new <FullParamSpacePanel> on /proposals/[id] that
renders every parameter the proposal's template declares, partitioned
into three visually distinct groups derived client-side from data
already on the page:

  1. Tuned (changed by this proposal) — appears in proposal.config_diff,
     rendered with from→to delta via the existing extractFromTo helper
     (promoted to ui/src/lib/config-diff.ts per FR-5).
  2. Tuned (unchanged) — in study.search_space.params but not in
     config_diff (the optimizer considered it but the digest's
     recommended_config didn't include it).
  3. Not in search space — declared on the template but absent from
     this study's tuning surface.

The pure helper partitionTemplateParams in ui/src/lib/proposal-param-space.ts
owns the partition algorithm; <FullParamSpacePanel> is a thin renderer.
Both are unit-testable without DOM.

Backend: NONE. Migration: NONE. The feature consumes existing
endpoints (proposals/{id}, studies/{id}, query-templates/{id}) only.

Cross-model review highlights (3 cycles, 19 findings total — 18
accepted+applied, 1 rejected with cited counter-evidence):
  - Cycle 3 F1 (High, accepted): caught a real correctness bug — the
    earlier FR-3 only lifted useTemplate's gate, not useStudy's, so
    study proposals with text-only digests would have mounted the
    panel with searchSpaceParams undefined and mis-classified every
    search-space param as 'untuned' instead of 'tunedUnchanged'.
    FR-3 + D-13 now require lifting BOTH fetches.
  - Cycle 2 F2 (High, accepted): FR-4 race-aware gating contradicted
    the section 11 narrative; aligned (panel waits for both parentTemplate
    + parentStudy.isPending=false before mounting for study-backed cases).
  - Cycle 1 F8 (Medium, rejected): GPT-5.5 worried the lifted
    useTemplate would change <SuggestedFollowupsPanel>'s rendering for
    previously-disabled cases. Rejected with counter-evidence at
    suggested-followups-panel.tsx:90-95+119-130 — parentTemplate is
    structurally consumed only by <SwapTemplateCard>, so non-
    swap-template proposals are indifferent to the prop.

Caps 2 + 3 from the idea (cross-panel hover linking + study-detail
mount) are explicitly deferred WITHOUT phase*_idea.md artifacts
(D-8 + D-14) — reopen only if specific operator feedback surfaces.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>

* docs(plan): feat_proposal_full_param_space_view — 4-story plan with lifted-fetch race-aware mount

Generates the implementation plan from the approved spec. Converged
across 3 GPT-5.5 cycles + 1 Opus internal verification pass — 19
findings total, 18 accepted+applied, 1 rejected with cited counter-
evidence.

Plan structure: 1 epic, 4 stories, dependency-ordered (story number IS
execution order):

  Story 1.1 — Promote extractFromTo + renderValue to ui/src/lib/config-diff.ts
              (shared helper extraction, 7 unit cases, AC-9 byte-identical
              preservation of ConfigDiffPanel rendering).
  Story 1.2 — Pure helper partitionTemplateParams (the FR-1 partition
              algorithm — 8 unit tests covering AC-1/2/3/5/6, D-9 search-
              space drift drop, D-10 from===to anomaly, sort stability).
  Story 1.3 — <FullParamSpacePanel> component + new proposal.full_param_space
              glossary key (7 component vitest tests, AC-1/2/5/6/7/8).
  Story 1.4 — Page-level integration: lift BOTH useTemplate AND useStudy
              gates (drop hasActionableFollowup); race-aware conditional
              mount; 6 page-level vitest tests + 1 real-backend
              Playwright E2E test.

Cross-model review highlights:
  - Cycle 1 F7 (Low, rejected): GPT-5.5 worried seedManualProposal
    wasn't a real helper. Counter-evidence: it's defined locally at
    proposals.spec.ts:21-36 as a 3-helper composition.
  - Cycle 3 F1 (High, accepted): caught a TypeScript build-breaker —
    Object.keys + indexed access on Record<string, string> with the
    project's noUncheckedIndexedAccess gate yields string | undefined.
    Algorithm now uses Object.entries (type-narrowed) + ?? '(unknown)'
    fallback for the AC-6 drift path.
  - Cycle 3 F6 (Medium, accepted): missing dedicated test for FR-7
    edge case A (source-study fetch error). Added as page-level Test 6.
  - Cycle 1 F2 (High, accepted): the cycle-3 F1 regression guard was
    bundled into the happy-path test which uses swap_template (already
    actionable, so wouldn't catch the bug). Split into dedicated
    Test 5 with empty/text-only digest.
  - Cycle 3 F5 (Medium, accepted): Test 4 race-gating used single
    deferred resolver — could pass vacuously if template was also
    pending. Switched to dual-deferred + qc.getQueryState confirmation.

No backend changes, no migrations, no audit events. Plan is fully
frontend, single-phase, no phase*_idea.md artifacts per D-14.

Total test coverage: 7 (config-diff unit) + 8 (partition unit) +
7 (panel component) + 6 (page-level vitest) + 1 (real-backend
Playwright E2E) = 29 new tests. Existing tests stay byte-identical
(AC-9 + AC-10 enforce this).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>

* refactor(proposals): promote extractFromTo + renderValue to shared module (Story 1.1)

Extract the two config_diff value-rendering helpers from
config-diff-panel.tsx into ui/src/lib/config-diff.ts so the new
<FullParamSpacePanel> (Story 1.3) can reuse the same canonical
{from, to}-vs-2-tuple normalization without duplication.

- New ui/src/lib/config-diff.ts exports extractFromTo + renderValue.
- config-diff-panel.tsx re-imports both; rendering byte-identical (AC-9).
- New config-diff.test.ts: 7 cases (3 extractFromTo + 4 renderValue).
- Existing config-diff-panel.test.tsx passes unchanged (6 tests).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>

* feat(proposals): pure partitionTemplateParams helper (Story 1.2)

The FR-1 partition algorithm — partitions a template's declared params
into tunedChanged / tunedUnchanged / untuned given the proposal's
config_diff + the source study's search_space.params.

- Partition universe is declaredParams union configDiff (D-9); search-space-
  only drift keys are silently dropped.
- config_diff membership is the operational definition of "tuned" (D-10:
  a from===to anomaly still classifies as tunedChanged).
- Drift keys (in config_diff, not in declared_params) render type
  '(unknown)' (AC-6).
- Uses Object.entries (type-narrowed) to satisfy noUncheckedIndexedAccess.
- 8 unit tests covering AC-1/2/3/5/6, D-9, D-10, sort stability.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>

* feat(proposals): FullParamSpacePanel component + glossary key (Story 1.3)

A new <FullParamSpacePanel> renders the three-state partition
(tuned-changed / tuned-unchanged / not-in-search-space) for a
proposal's template, consuming the pure partitionTemplateParams helper.

- Card shell + InfoTooltip matching the existing proposal-panel pattern.
- Three labeled groups, each omitted when empty; full-universe-empty
  shows the param-space-empty state (declaredParams AND config_diff
  both empty — AC-6 drift path takes precedence otherwise).
- tunedChanged rows show from→to; tunedUnchanged "(no change)"; untuned
  italic — reusing <DeclaredParamsColumn>'s typography.
- New proposal.full_param_space glossary key (FR-6).
- 7 component tests (AC-1/2/5/6/7/8 + full-empty defensive); glossary
  AC-12 audience-language check passes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>

* feat(proposals): mount FullParamSpacePanel with lifted fetches + race-aware gating (Story 1.4)

Page-level integration of the full-parameter-space panel on
/proposals/[id].

- Lift BOTH useTemplate (now sourced from proposal.template.id, null-safe)
  AND useStudy (drop the `&& hasActionableFollowup` gate) so the panel
  has declared_params + search_space.params for EVERY proposal shape
  (FR-3 / D-13). Removed the now-dead hasActionableFollowup variable.
- Mount the panel below ConfigDiffPanel with race-aware gating: wait for
  parentTemplate.data AND, for study-backed proposals, parentStudy
  settled (FR-4) so the tunedUnchanged group never flashes a transient
  mis-classification.
- 6 new page-level vitest tests: happy path, manual proposal, template
  404, race-gating (dual-deferred resolver), FR-3 regression guard
  (no-actionable-followups digest), FR-7 edge A (study fetch error).
- 1 new real-backend Playwright E2E asserting the panel renders for a
  seeded manual proposal.

All 18 page tests pass (12 existing + 6 new). All 5 proposals E2E pass
(verified against a rebuilt production container). tsc + build clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>

* test(proposals): strengthen tests per GPT-5.5 phase-gate review

Phase-gate cross-model review findings (4 accepted, 1 rejected):

- F1 (accepted): D-9 search-space-drift unit test now uses
  searchSpaceParams={phantom} only, asserting `foo` (declared, not in
  search space) → untuned AND phantom dropped — covers the classification
  the prior version skipped by including foo in search space.
- F2 (accepted): component AC-1 test now asserts the group-header count
  text ("2 parameters" / "1 parameter") per FR-2, not just the testids.
- F3 (accepted): page template-404 test now asserts <PrPanel>
  (open-pr-button) stays visible alongside ConfigDiffPanel + metric-delta.
- F4 (accepted): race-gating test converted to the documented
  dual-deferred resolver pattern (both template + study deferred) to
  eliminate any vacuous-pass window.
- F5 (rejected): GPT wanted an exhaustive switch(group)/never default;
  counter-evidence — GROUP_LABELS: Record<ParamSpaceGroup, string> in
  full-param-space-panel.tsx already gives compile-time exhaustiveness
  (a 4th variant is a type error at the literal).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>

* docs(guides): regenerate guide 02 screenshots for the full-parameter-space panel

The new <FullParamSpacePanel> mounts on /proposals/[id] between the
config-diff and metric-delta panels, so guide 02's proposal-detail
screenshot (03-proposal-detail.png) now shows it. Regenerated the full
guide-02 screenshot set against the running stack so the walkthrough
reflects the shipped UI.

03-proposal-detail.png confirms the panel renders correctly end-to-end:
config_diff drift keys (description.boost / title.boost, type
"(unknown)" since the seeded template declares only `boost`) under
"Tuned (changed by this proposal)", and `boost` under "Not in search
space". The other four PNGs changed only from re-seeded demo data.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>

* fix(proposals): null-safe search-space guard + grid-aligned tuned rows (Gemini review)

Gemini Code Assist adjudication (both accepted):

- G1 (High): `searchSpaceParams !== undefined && key in searchSpaceParams`
  throws TypeError when searchSpaceParams is null (null !== undefined is
  true, and `key in null` throws). JSONB study.search_space.params can be
  null at runtime. Fixed with a truthiness guard + widened the
  PartitionInput/prop type to Record | null | undefined to be honest about
  the runtime contract. Added a null-search-space regression unit test.
- G2 (Medium): tunedChanged rows now use a CSS grid so name/type/from/→/to
  align vertically across rows (matching ConfigDiffPanel's table columns).
  Tests are layout-agnostic and stay green.

34 lib+component+page tests pass; tsc + build + lint clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>

* test(proposals): close race-gating coverage hole (GPT-5.5 final review FF1)

The AC-11 race-gating test asserted that tuned_unchanged + empty were
absent during the template-ready/study-pending window — but BOTH are
absent even on a premature mount (config_diff empty + no search space →
foo classifies as `untuned`, not tuned_unchanged or empty). So the guard
would have passed even if the panel mounted too early.

Add assertions that param-space-group-untuned AND
param-space-row-untuned-foo are also absent during the race window —
these WOULD render on a premature mount, so the test now genuinely
catches the race bug FR-4's gating defends against.

(GPT-5.5 final review FF2 — "ACTIONABLE_FOLLOWUP_KINDS unused" — rejected:
still consumed at page.tsx:196 in the prefillValues useMemo; tsc + lint
clean.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>

---------

Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant